Often I end up dealing with code that works but might not be of the highest quality. While quality is subjective I like to use the idea of
"code smell" to convey what I mean, these are a list of indicators that, in total, help to identify code that might benefit from some improvement.
Such smells may include:
- Complex code lacking comments on intended operation
- Code lacking API documentation comments especially for interfaces used outside the local module
- Not following style guide
- Inconsistent style
- Inconsistent indentation
- Poorly structured code
- Overly long functions
- Excessive use of pre-processor
- Many nested loops and control flow clauses
- Excessive numbers of parameters
I am most certainly not alone in using this approach and
Fowler et al have covered this subject in the literature much better than I can here. One point I will raise though is some programmers dismiss code that exhibits these traits as "legacy" and immediately suggest a fresh implementation. There are varying opinions on when a rewrite is the appropriate solution from
never to always but in my experience making the old working code smell nice is almost always less effort and risk than a re-write.
TestsWhen I come across smelly code, and I decide it is worthwhile improving it, I often discover the biggest smell is lack of test coverage. Now do remember this is just one code smell and on its own might not be indicative, my experience is smelly code seldom has effective test coverage while fresh code often does.
Test coverage is generally understood to be the percentage of source code lines and decision paths used when instrumented code is exercised by a set of tests. Like many metrics developer tools produce, "coverage percentage" is often misused by managers as a proxy for code quality. Both
Fowler and
Marick have written about this but sufficient to say that for a developer test coverage is a useful tool but should not be misapplied.
Although refactoring without tests is possible the chances for unintended consequences are proportionally higher. I often approach such a refactor by enumerating all the callers and constructing a description of the used interface beforehand and check that that interface is not broken by the refactor. At which point is is probably worth writing a unit test to automate the checks.
Because of this I have changed my approach to such refactoring to start by ensuring there is at least basic API code coverage. This may not yield the fashionable 85% coverage target but is useful and may be extended later if desired.
It is widely known and equally widely ignored that for maximum effectiveness unit tests must be run frequently and developers take action to rectify failures promptly. A test that is not being run or acted upon is a waste of resources both to implement and maintain which might be better spent elsewhere.
For projects I contribute to frequently I try to ensure that the CI system is running the coverage target, and hence the unit tests, which automatically ensures any test breaking changes will be highlighted promptly. I believe the slight extra overhead of executing the instrumented tests is repaid by having the coverage metrics available to the developers to aid in spotting areas with inadequate tests.
ExampleA short example will help illustrate my point. When a web browser receives an object over HTTP the server can supply a MIME type in a content-type header that helps the browser interpret the resource. However this meta-data is often problematic (sorry that should read "a misleading lie") so the actual content must be examined to get a better answer for the user. This is known as mime sniffing and of course there is a living
specification.
The
source code that provides this API (Linked to it rather than included for brevity) has a few smells:
- Very few comments of any type
- The API are not all well documented in its header
- A lot of global context
- Local static strings which should be in the global string table
- Pre-processor use
- Several long functions
- Exposed API has many parameters
- Exposed API uses complex objects
- The git log shows the code has not been significantly updated since its implementation in 2011 but the spec has.
- No test coverage
While some of these are obvious the non-use of the global string table and the API complexity needed detailed knowledge of the codebase, just to highlight how subjective the sniff test can be. There is also one huge air freshener in all of this which definitely comes from experience and that is the modules author. Their name at the top of this would ordinarily be cause for me to move on, but I needed an example!
First thing to check is the API use
$ git grep -i -e mimesniff_compute_effective_type --or -e mimesniff_init --or -e mimesniff_fini
content/hlcache.c: error = mimesniff_compute_effective_type(handle, NULL, 0,
content/hlcache.c: error = mimesniff_compute_effective_type(handle,
content/hlcache.c: error = mimesniff_compute_effective_type(handle,
content/mimesniff.c:nserror mimesniff_init(void)
content/mimesniff.c:void mimesniff_fini(void)
content/mimesniff.c:nserror mimesniff_compute_effective_type(llcache_handle *handle,
content/mimesniff.h:nserror mimesniff_compute_effective_type(struct llcache_handle *handle,
content/mimesniff.h:nserror mimesniff_init(void);
content/mimesniff.h:void mimesniff_fini(void);
desktop/netsurf.c: ret = mimesniff_init();
desktop/netsurf.c: mimesniff_fini();
This immediately shows me that this API is used in only a very small area, this is often not the case but the general approach still applies.
After a little investigation the usage is effectively that the mimesniff_init API must be called before the mimesniff_compute_effective_type API and the mimesniff_fini releases the initialised resources.
A simple test case was added to cover the API, this exercised the behaviour both when the init was called before the computation and not. Also some simple tests for a limited number of well behaved inputs.
By changing to using the global string table the initialisation and finalisation API can be removed altogether along with a large amount of global context and pre-processor macros. This single change removes a lot of smell from the module and raises test coverage both because the global string table already has good coverage and because there are now many fewer lines and conditionals to check in the mimesniff module.
I stopped the refactor at this point but were this more than an example I probably would have:
- made the compute_effective_type interface simpler with fewer, simpler parameters
- ensured a solid set of test inputs
- examined using a fuzzer to get a better test corpus.
- added documentation comments
- updated the implementation to 2017 specification.
ConclusionThe approach examined here reduce the smell of code in an incremental, testable way to improve the codebase going forward. This is mainly necessary on larger complex codebases where technical debt and bit-rot are real issues that can quickly overwhelm a codebase if not kept in check.
This technique is subjective but helps a programmer to quantify and examine a piece of code in a structured fashion. However it is only a tool and should not be over applied nor used as a metric to proxy for code quality.
It's (yet again!) been a while since I blogged last, sorry...
It's been over ten years since I started
in Arm, and nine since I joined
Linaro as an assignee. It was
wonderful working with some excellent people in both companies, but around
the end of last year I started to think that it might be time to look
for something new and different. As is the usual way in Cambridge, I
ended up mentioning this to friends and things happened!
After discussions with a few companies, I decided to accept an
interesting-looking offer from a Norwegian company called Pexip. My good friend Vince had been raving
for a while about how much he enjoyed his job there, which was a very
good sign! He works from his home near Cambridge, and they were very
happy to take me on in a similar way. There will be occasional trips
to the UK office near Reading, or to the Norway HQ in Oslo. But most
of the time I'll be working in my home office with all the home
comforts and occasionally even an office dog!
As is common in the UK for senior staff, I had to give 3 months
notice with my resignation. When I told my boss in Arm
way way back in February that I had decided to leave,
I planned for a couple of weeks of down-time in between jobs. Perfect
timing! The third week of May in Cambridge is the summer Beer
Festival, and my birthday is the week after. All was looking good!
Then the world broke... :-(
As the "novel coronavirus" swept the world, countries closed down
and normal life all-but disappeared for many. I acknowledge I'm very
lucky here - I'm employed as a software engineer. I can effectively
work from home, and indeed I was already in the habit of doing that
anyway. Many people are not so fortunate. :-/ In this period, I've
heard of some people in the middle of job moves where their new
company have struggled and the new job has gone away. Thankfully,
Pexip have continued to grow during this time and were still very keen
to have me. I finally started this week!
So, what does Pexip do? The company develops and supplies a video
conferencing platform, mainly targeting large enterprise customers. We
have some really awesome technology, garnering great reviews from
customers all over the world. See the website for more information!
Where do I fit in? Pexip is a relatively small company with a very
flat setup in engineering, so that's a difficult question to answer!
I'll be starting working in the team developing and maintaining PexOS,
the small Linux-based platform on which other things depend. (No
prizes for guessing which distro it's based on!) But there's lots of
scope to get involved in all kinds of other areas as needs and
interests arise. I can't wait to get stuck in!
Although I'm no longer going to be working on Debian arm port
issues on work time, I'm still planning to help where I can. Let's see
how that works...
A common way to establish an IPsec tunnel on Linux is to use an 
A possible configuration between 
For more in-depth explanation on the different ways to encode a routing table
into a trie and a better understanding of radix trees, see
the 
The 
Getting meaningful results is challenging due to the size of the address
space. None of the scenarios have a fallback route and we only measure time for
successful hits
Only 50% of the time is spent in the actual route lookup. The remaining time is
spent evaluating the routing rules (about 30 ns). This ratio is dependent on the
number of routes we inserted (only 1000 in this example). It should be noted the
Here is an approximate breakdown on the time spent:
All kernels are compiled with 
Despite its more complex insertion logic, the IPv4 subsystem is able to insert 2
million routes in less than 10 seconds.
The memory usage is therefore quite predictable and reasonable, as even a small
single-board computer can support several full views (20 MiB for each):
The 
Welcome to gambaru.de. Here is my monthly report that covers what I have been doing for Debian. If you re interested in Java, Games and LTS topics, this might be interesting for you.
Debian Games
Two scenarios are tested:
For each node, the prefix is known by its path from the root node and
the prefix length is the current depth.
A lookup in such a trie is quite simple: at each step, fetch the
nth bit of the 
Since some bits have been ignored, on a match, a final check is
executed to ensure all bits from the found entry are matching the
input 
The reduction on the average depth of the tree compensates the
necessity to handle those false positives. The insertion and deletion
of a routing entry is still easy enough.
Many routing systems are using Patricia trees:
Such a trie is called 
There are several structures involved:
The lookup time is loosely tied to the maximum depth. When the routing
table is densily populated, the maximum depth is low and the lookup
times are fast.
When forwarding at 10 Gbps, the time budget for a packet would be
about 50 ns. Since this is also the time needed for the route lookup
alone in some cases, we wouldn t be able to forward at line rate with
only one core. Nonetheless, the results are pretty good and they are
expected to scale linearly with the number of cores.
The measurements are done with a Linux kernel 4.11 from Debian
unstable. I have gathered performance metrics accross kernel versions
in 
The results are quite good. With only 256 MiB, about 2 million routes
can be stored!
For some reason, the relation is linear when the number of rules is
between 1 and 100 but the slope increases noticeably past this
threshold. The second graph highlights the negative impact of the
first rule (about 30 ns).
A common use of rules is to create virtual routers: interfaces
are segregated into domains and when a datagram enters through an
interface from domain A, it should use routing table A:
In the above example, we have hypervisors hosting a virtual
machines from different tenants. Each virtual machine is given
access to a tenant-specific virtual Ethernet segment. Users are
expecting classic Ethernet segments: no 
Compared to 
VXLANs can be provisioned with the following script. Source-address
learning is disabled as we will rely solely on 
To illustrate our examples, we use the following setup:
The 
The main expectation of such a setup is that while the virtual hosts
should be able to use resources from the public network, they should
not be able to access resources from the infrastructure network
(including resources hosted on the hypervisor itself, like a 
